-
Notifications
You must be signed in to change notification settings - Fork 3.2k
App Configuration - Snapshot references #44116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds support for snapshot references in the Azure App Configuration Provider SDK for Python. Snapshot references allow configuration settings to reference entire snapshots, which are then resolved and merged into the configuration at load time. This feature enables better configuration management by allowing users to group related settings into snapshots and reference them dynamically.
Key Changes:
- Introduced snapshot reference parsing and resolution functionality with recursive processing support
- Added telemetry tracking for snapshot reference usage via the request tracing context
- Extended both synchronous and asynchronous providers to handle snapshot reference resolution
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
_constants.py |
Added constants for snapshot reference MIME profile, content type, and telemetry tag |
_snapshot_reference_parser.py |
New parser class for validating and extracting snapshot names from reference JSON |
_client_manager.py |
Added resolve_snapshot_reference method to resolve snapshot references to actual settings |
aio/_async_client_manager.py |
Async version of snapshot reference resolution functionality |
_azureappconfigurationprovider.py |
Modified _process_configurations to detect and resolve snapshot references recursively |
aio/_azureappconfigurationproviderasync.py |
Async version of snapshot reference processing in configuration loading |
_azureappconfigurationproviderbase.py |
Added telemetry tracking for snapshot reference content type detection |
_request_tracing_context.py |
Added uses_snapshot_reference flag and correlation context header support |
test_snapshot_references.py |
Unit tests for snapshot reference parser validation logic |
test_snapshot_references_integration.py |
Integration tests for snapshot reference parsing and content type detection |
aio/test_async_snapshot_references_integration.py |
Async integration tests for snapshot reference resolution in client manager and provider |
test_request_tracing_context.py |
Tests for snapshot reference telemetry tracking in correlation context |
refresh_sample.py |
Unrelated sample modifications that should be reverted |
...guration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py
Show resolved
Hide resolved
...-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py
Show resolved
Hide resolved
...pconfiguration/azure-appconfiguration-provider/tests/test_snapshot_references_integration.py
Show resolved
Hide resolved
...iguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/azure-appconfiguration-provider/tests/test_request_tracing_context.py
Outdated
Show resolved
Hide resolved
| # Recursively process the resolved snapshot settings to handle feature flags, | ||
| # configuration mapping | ||
| snapshot_configuration_list = list(snapshot_settings.values()) | ||
| resolved_settings = await self._process_configurations(snapshot_configuration_list, client) |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The recursive call to _process_configurations on line 473 could lead to infinite recursion if a snapshot contains a snapshot reference that points back to itself or creates a circular reference chain. There's no depth tracking or cycle detection to prevent this.
Consider adding a depth limit parameter or cycle detection mechanism to prevent stack overflow from circular snapshot references.
sdk/appconfiguration/azure-appconfiguration-provider/tests/test_request_tracing_context.py
Outdated
Show resolved
Hide resolved
...configuration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py
Outdated
Show resolved
Hide resolved
...-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py
Outdated
Show resolved
Hide resolved
...guration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
…configuration/provider/_azureappconfigurationproviderbase.py Co-authored-by: Copilot <[email protected]>
| APP_CONFIG_SNAPSHOT_REF_MIME_PROFILE = "https://azconfig.io/mime-profiles/snapshot-ref" | ||
|
|
||
| # Snapshot reference tracing key | ||
| SNAPSHOT_REFERENCE_TAG = "UsesSnapshotReference=true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snapshot ref is part of the features string. Eg: "Features=AI+SnapshotRef"
| # Mime profiles | ||
| APP_CONFIG_AI_MIME_PROFILE = "https://azconfig.io/mime-profiles/ai/" | ||
| APP_CONFIG_AICC_MIME_PROFILE = "https://azconfig.io/mime-profiles/ai/chat-completion" | ||
| APP_CONFIG_SNAPSHOT_REF_MIME_PROFILE = "https://azconfig.io/mime-profiles/snapshot-ref" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to track MIME profile - just compare with SNAPSHOT_REF_CONTENT_TYPE to check if it's a snapshot reference. We do the same in .net provider: https://github.com/Azure/AppConfiguration-DotnetProvider/blob/34576ef70d303a029b7ea7ef860f12885fb8f35d/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs#L869C45-L869C57
|
|
||
| if self._feature_flag_refresh_enabled: | ||
| self._watched_feature_flags[(settings.key, settings.label)] = settings.etag | ||
| elif settings.content_type and SNAPSHOT_REF_CONTENT_TYPE in settings.content_type: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| elif settings.content_type and SNAPSHOT_REF_CONTENT_TYPE in settings.content_type: | |
| elif settings.content_type and SNAPSHOT_REF_CONTENT_TYPE == settings.content_type: |
| APP_CONFIG_AICC_MIME_PROFILE = "https://azconfig.io/mime-profiles/ai/chat-completion" | ||
|
|
||
| # Snapshot reference tracing key | ||
| SNAPSHOT_REFERENCE_TAG = "UsesSnapshotReference" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SNAPSHOT_REFERENCE_TAG = "UsesSnapshotReference" | |
| SNAPSHOT_REFERENCE_TAG = "SnapshotRef" |
| # Snapshot reference tracing key | ||
| SNAPSHOT_REFERENCE_TAG = "UsesSnapshotReference" | ||
|
|
||
| # Snapshot reference constants |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file contains all the constants. Could you organize them into sections so that related constants are grouped together? Eg tracing constants, content types, FM constants.
| or empty/whitespace snapshot name | ||
| """ | ||
| if setting is None: | ||
| raise ValueError("Setting cannot be None") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does ValueError get handled properly if the provider is optional?
| if self.is_failover_request: | ||
| tags.append(FAILOVER_TAG) | ||
|
|
||
| if self.uses_snapshot_reference: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldnt this be part of features_string?
| snapshot_selector = SettingSelector(snapshot_name=snapshot_name) | ||
|
|
||
| # Use existing load_configuration_settings to load from snapshot | ||
| configurations = self.load_configuration_settings([snapshot_selector], **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If snapshot contains feature flags, will those be ignored?
| if APP_CONFIG_AICC_MIME_PROFILE in config.content_type: | ||
| self._tracing_context.uses_aicc_configuration = True | ||
| if SNAPSHOT_REF_CONTENT_TYPE in config.content_type: | ||
| self._tracing_context.uses_snapshot_reference = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already tracked in _process_configurations, do we still need to check content type here?
| self._tracing_context.uses_snapshot_reference = True | ||
| try: | ||
| # Resolve the snapshot reference to actual settings | ||
| snapshot_settings = client.resolve_snapshot_reference(settings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snapshot references should be resolved before _configuration_mapper is run - i.e. _configuration_mapper runs on all the settings loaded by snapshot or snapshot reference
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines